Skip to content

fix(security): patch copilot tool & multipart upload IDORs#4304

Merged
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/fix-copilot-idor
Apr 27, 2026
Merged

fix(security): patch copilot tool & multipart upload IDORs#4304
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/fix-copilot-idor

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Multipart upload: bind upload session to (userId, workspaceId, key) via short-lived HMAC-signed token; require workspace write access at initiate; source key/uploadId/context from the verified token (never client) at get-part-urls/complete/abort
  • Copilot knowledge-base tools: gate all 11 read/write/tag/connector ops with checkKnowledgeBaseAccess / checkKnowledgeBaseWriteAccess
  • Copilot user-table tools: add workspace-id check to get, get_schema, add/rename/delete/update_column to match existing op pattern
  • Copilot manage-credential: add full ownership/write-permission auth via getCredentialActorContext (previously had no auth)
  • Copilot restore-resource: verify workspace ownership and write permission for workflow, table, knowledgebase, file, and folder restores
  • Copilot folder rename/move: verify both folderId and parentId belong to the caller's workspace via new verifyFolderWorkspace helper
  • Copilot get-job-logs: verify the schedule belongs to the caller's workspace before returning logs

Type of Change

  • Bug fix

Testing

Tested manually. Typecheck clean and full test suite (5337 tests) passing.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

- multipart upload: bind upload session to (userId, workspaceId, key)
  via short-lived HMAC-signed token; require workspace write access at
  initiate; source key/uploadId/context from verified token (never
  client) at get-part-urls/complete/abort
- copilot knowledge-base tools: gate all 11 read/write/tag/connector
  ops with checkKnowledgeBaseAccess / checkKnowledgeBaseWriteAccess
- copilot user-table tools: add workspace-id check to get, get_schema,
  add/rename/delete/update_column to match existing op pattern
- copilot manage-credential: add full ownership/write-permission auth
  via getCredentialActorContext (previously had no auth)
- copilot restore-resource: verify workspace ownership and write
  permission for workflow, table, knowledgebase, file, and folder
  restores
- copilot folder rename/move: verify both folderId and parentId belong
  to the caller's workspace via new verifyFolderWorkspace helper
- copilot get-job-logs: verify schedule belongs to caller's workspace
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 27, 2026 5:40pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 27, 2026

PR Summary

Medium Risk
Tightens authorization and adds token-binding on upload and copilot endpoints; risk is moderate because stricter checks and new required parameters/tokens can break existing callers and block previously-allowed operations if assumptions were incorrect.

Overview
Security hardening across uploads and copilot tools to prevent IDORs. Multipart upload now requires workspaceId + workspace write/admin permission at initiation, returns an HMAC-signed short-lived uploadToken, and all follow-up actions (get-part-urls, complete, abort) derive uploadId/key/context from the verified token (with an allowlist of valid StorageContexts) instead of trusting client input.

Copilot tools now enforce workspace-scoped access before mutating or returning data: credential rename/delete is gated via getCredentialActorContext; restore operations validate the target resource’s workspace and require write/admin; workflow folder move/rename validates folder ownership via new verifyFolderWorkspace; job logs queries require workspace context and filter by workspaceId; knowledge-base and user-table tools add explicit read/write checks to prevent cross-workspace access.

Reviewed by Cursor Bugbot for commit 3f7a3ba. Configure here.

Comment thread apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR patches a set of IDOR and missing-authorization vulnerabilities across the multipart upload API and a wide range of copilot tool handlers. The multipart upload fix is particularly well-designed: sessions are now bound to a short-lived HMAC-SHA256-signed token (uploadId + key + userId + workspaceId + context) issued only after a workspace write-permission check, and all follow-up actions source their context exclusively from the verified token rather than trusting the client.

The copilot handler fixes are comprehensive — knowledge-base (11 ops), user-table (6 ops), manage-credential, restore-resource, folder move/rename, and get-job-logs are all gated with appropriate access checks. The logic throughout is consistently fail-closed. The only minor concern is the fixed 1-hour upload-token TTL, which could cause token expiry for very large uploads before the session completes.

Confidence Score: 5/5

Safe to merge — all IDORs are patched with fail-closed logic and no new regressions introduced

No P0 or P1 findings. The security fixes are correct and comprehensive; the only open item is a P2 TTL concern for very large uploads. Previous review threads confirm earlier edge cases (split limit, log count) were addressed in a prior commit.

No files require special attention; upload-token.ts TTL default is worth a second look if multi-hour large file uploads are a real-world scenario

Important Files Changed

Filename Overview
apps/sim/app/api/files/multipart/route.ts Binds upload sessions to HMAC-signed tokens; initiate now requires workspace write access; get-part-urls/complete/abort source context from the verified token only
apps/sim/lib/uploads/core/upload-token.ts New file: well-implemented HMAC-SHA256 signed upload token; validates token structure strictly on verify with exact 2-part split enforcement
apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts All 11+ read/write/tag/connector operations now gated with checkKnowledgeBaseAccess or checkKnowledgeBaseWriteAccess; per-document check for delete_documents and update_document
apps/sim/lib/copilot/tools/handlers/restore-resource.ts Adds workspace ownership + write-permission checks for workflow, table, knowledgebase (fetched from DB), and file/folder (caller workspace + service-layer scoping)
apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts executeMoveWorkflow verifies folderId belongs to caller's workspace; executeMoveFolder and executeRenameFolder both verify folder/parentId via verifyFolderWorkspace; null workspaceId correctly fails closed
apps/sim/lib/copilot/tools/server/jobs/get-job-logs.ts Workspace context is now required (fail-closed); workspaceId added as a DB filter condition preventing cross-workspace log leakage
apps/sim/lib/copilot/tools/handlers/management/manage-credential.ts Auth added via getCredentialActorContext; checks hasWorkspaceAccess and canWriteWorkspace/isAdmin for both rename and delete operations
apps/sim/lib/copilot/tools/server/table/user-table.ts Workspace-id ownership check added to get, get_schema, add_column, rename_column, delete_column, update_column to match existing row-ops pattern
apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts Client updated to send workspaceId at initiate and use uploadToken (not raw uploadId/key) for subsequent requests
apps/sim/lib/workflows/utils.ts New verifyFolderWorkspace helper queries workflowFolder by (id, workspaceId) to prevent folder IDOR in move/rename operations

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant API as Multipart API
    participant Auth as Workspace ACL
    participant Token as HMAC Signer
    participant Store as Cloud Storage

    C->>API: initiate {fileName, workspaceId, context}
    API->>Auth: check write permission
    Auth-->>API: allowed
    API->>Store: start upload session
    Store-->>API: uploadId + path
    API->>Token: sign session payload (1h TTL)
    Token-->>API: uploadToken
    API-->>C: uploadId, path, uploadToken

    C->>API: get-part-urls {uploadToken, partNumbers}
    API->>Token: verify token + userId
    Token-->>API: session payload
    API->>Store: generate presigned part URLs
    API-->>C: presignedUrls

    C->>Store: PUT chunks via presigned URLs

    C->>API: complete {uploadToken, parts}
    API->>Token: verify token + userId
    Token-->>API: session payload
    API->>Store: finalize upload
    API-->>C: success + location
Loading

Reviews (3): Last reviewed commit: "fix(security): close folder workspace by..." | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/tools/handlers/restore-resource.ts
Comment thread apps/sim/app/api/files/multipart/route.ts
Comment thread apps/sim/lib/uploads/core/upload-token.ts Outdated
- knowledge-base delete_document/update_document: verify each document
  belongs to the claimed knowledgeBaseId via checkDocumentWriteAccess
  (was: trusted args.knowledgeBaseId without binding it to the document)
- multipart batch complete: log verifiedEntries.length instead of raw
  client-supplied data.uploads.length
- upload-token: reject tokens with !=2 dot-delimited segments
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts
Comment thread apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3f7a3ba. Configure here.

@waleedlatif1 waleedlatif1 merged commit c32c1cb into staging Apr 27, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/fix-copilot-idor branch April 27, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant